-
Notifications
You must be signed in to change notification settings - Fork 11
feat(react-journey): add react journey app #86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
javascript/react-journey/README.md
Outdated
| ## Requirements | ||
|
|
||
| 1. An instance of Ping's Access Manager (AM), either within a Ping's Advanced Identity Cloud tenant, your own private installation or locally installed on your computer | ||
| 2. Node >= 14.2.0 (recommended: install via [official package installer](https://nodejs.org/en/)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should update this to a newer node, we dont test this far back so maybe 18? 18 is still pretty old
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to 18.12.0
ryanbas21
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass
javascript/react-journey/client/components/journey/identity-provider.js
Outdated
Show resolved
Hide resolved
| */ | ||
|
|
||
| // TODO: Have we ported over HttpClient yet? | ||
| import { HttpClient } from '@forgerock/javascript-sdk'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We haven't but i think we could probably remove this and use a plain fetch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not a big fan of module level "globals", not sure what else they would be called. They come across like a smell to me.
We could adopt a Reader like pattern to achieve this. Reader is the functional programming jargon for how to keep environment state inside a closure (more or less).
import { oidc, type OidcClient } from '@forgerock/oidc-client';
import createConfig from './create-config.js';
/**
* Creates an OIDC client instance. The caller is responsible for
* threading this through the application (e.g., via React context).
*/
export async function createOidcClient(): Promise<OidcClient> {
const config = createConfig();
return oidc({ config });
}
/**
* Type for functions that need access to the OIDC client.
* This is the "Reader" pattern - functions declare their dependency
* and receive it as an argument rather than reaching into global state.
* Likely not used in a react app but possibly on functions/utilities if needed
*/
export type WithOidcClient<T> = (client: OidcClient) => T;Then we just use it in context like
import { createContext, useContext, type ReactNode } from 'react';
import type { OidcClient } from '@forgerock/oidc-client';
const OidcContext = createContext<OidcClient | null>(null);
export function useOidcClient(): OidcClient {
const client = useContext(OidcContext);
if (!client) {
throw new Error('useOidcClient must be used within OidcProvider');
}
return client;
}
export function OidcProvider({
client,
children
}: {
client: OidcClient;
children: ReactNode
}) {
return (
<OidcContext.Provider value={client}>
{children}
</OidcContext.Provider>
);
}Then in the App root it's something like:
function App() {
const [client, setClient] = useState<OidcClient | null>(null);
useEffect(() => {
createOidcClient().then(setClient);
}, []);
if (!client) return <Loading />;
return (
<OidcProvider client={client}>
<Router />
</OidcProvider>
);
}small example of how that dangling type may get used (may not also need to be used)
const fetchUserProfile: WithOidcClient<Promise<UserProfile>> = async (client) => {
const tokens = await client.getTokens();
return client.userinfo(tokens.accessToken);
};This keeps no hidden state in the module itself, and pushes that responsibility up to the composition root.
This also allows components to have more explicit dependency on the oidc client itself, rather than importing something from the module that may throw.
This is similar to in Effect how they use Layers as the composition model.
89231fe to
cc7c400
Compare
cc7c400 to
3962b82
Compare
Jira: https://pingidentity.atlassian.net/browse/SDKS-4047
Done: Supports basic username-password embedded login
TODO:
DO NOT MERGE: Pending release of journey client and oidc client. OIDC and Journey package dependencies need to be updated.